Skip to content

Potential fix for code scanning alert no. 12: Incomplete multi-character sanitization#29

Merged
n3m01726 merged 1 commit intomainfrom
alert-autofix-12
Mar 29, 2026
Merged

Potential fix for code scanning alert no. 12: Incomplete multi-character sanitization#29
n3m01726 merged 1 commit intomainfrom
alert-autofix-12

Conversation

@n3m01726
Copy link
Copy Markdown
Owner

Potential fix for https://github.com/n3m01726/discord-bot/security/code-scanning/12

In general: the problem arises because on\w+= can span multiple characters that, once partially removed, may expose a new on\w+= not seen in the first pass. To fix this without changing existing behavior, we can either (a) loop the replacements until the string stabilizes, or (b) avoid this brittle pattern entirely by restricting ourselves to simpler, character-based sanitization that cannot “reform” dangerous text. Since this is a low-level generic middleware and we must not change imports except for well-known libs, the minimal, robust fix is to make the sanitization idempotent by repeatedly applying the current replacements until no more changes occur.

Concretely, in src/api/middlewares/validation.js, inside sanitizeInput, we currently perform three chained .replace operations on the string. We will refactor this so that for string inputs, we first trim() once, then enter a do { ... } while (current !== previous) loop where, in each iteration, we apply the three .replace calls to the evolving string. When the string stops changing, we return it. This exactly preserves the intended semantics—removing <, >, javascript: and on\w+=—but closes the gap where an intermediate result could create a fresh on...= or javascript: sequence. No other parts of the file need to be changed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ter sanitization

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@n3m01726 n3m01726 marked this pull request as ready for review March 29, 2026 22:14
@n3m01726 n3m01726 merged commit 802670c into main Mar 29, 2026
2 checks passed
@n3m01726 n3m01726 deleted the alert-autofix-12 branch March 29, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant